Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add ArrayVec::as_inner() #197

Merged
merged 1 commit into from
Jul 8, 2024
Merged

Add ArrayVec::as_inner() #197

merged 1 commit into from
Jul 8, 2024

Conversation

Fuuzetsu
Copy link
Contributor

Originally I was going to call this as_array()
but as_inner() is more consistent with the
existing into_inner() method. I don't
ultimately care what it ends up being called.

I wonder if we should have a impl From<ArrayVec<T>> for A implementation or is that a bit too foot-gun-y?

Fixes #193.

@Fuuzetsu
Add ArrayVec::as_inner()
19b5c19 
Originally I was going to call this `as_array()`
but `as_inner()` is more consistent with the
existing `into_inner()` method. I don't
_ultimately_ care what it ends up being called.

I wonder if we should have a `impl
From<ArrayVec<T>> for A` implementation or is that
a bit too foot-gun-y?

This lives in its own `impl<A> ArrayVec<A>` block
because rust 1.47.0 is unhappy about having a
const function inside `impl<A: Array>
ArrayVec<A>`. We don't need the trait anyway.

Fixes Lokathor#193.
@Fuuzetsu
Copy link
Contributor Author

Fuuzetsu commented Jul 8, 2024

I updated the commit to make it compile on 1.47.

@Lokathor
Copy link
Owner

Lokathor commented Jul 8, 2024

I think as_inner makes more sense, because writing an array type to call from would be a pain in the butt and take extra angle brackets, compared to doing a method call on a vec you're working with.

@Lokathor Lokathor merged commit 1838a18 into Lokathor:main Jul 8, 2024
5 checks passed
@Lokathor
Copy link
Owner

Lokathor commented Jul 8, 2024

Released in tinyvec-1.8.0

@Fuuzetsu
Copy link
Contributor Author

Fuuzetsu commented Jul 10, 2024 •
edited
Loading

I think as_inner makes more sense, because writing an array type to call from would be a pain in the butt and take extra angle brackets, compared to doing a method call on a vec you're working with.

I meant having a From impl on top of having as_inner. However with the From impl, it's less obvious that some of the data is past the ArrayVec::len to make a mistake with it.

I think it's fine not to have it for now.

@LunNova
Copy link

LunNova commented Jul 10, 2024

Is it "safe" to access elements that are past the end of the vec because those elements are always the Default value rather than some undefined behavior, thanks to initializing with the default or mem::take being used when removing something from the Vec?

(I'm not trying to suggest there is a bug, just curious if I'm understanding why this is ok correctly)

@Lokathor
Copy link
Owner

Yes, the spare slots are not "in the list", but they're always initialized data, so it's safe to access them.

@Fuuzetsu
Copy link
Contributor Author

I didn't think we could rely on the elements past the "end" being the Default but maybe we can? If yes, can this get documented and made official? This is useful knowledge. I assumed some operations just leave the data as-is without dropping it. I thought truncate just changed the length value but apparently it really does drop things.

@Lokathor
Copy link
Owner

I'd have to check closely, but I believe that they will not always be the Default value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lokathor Lokathor Lokathor approved these changes

Assignees
No one assigned
Labels
semver-minor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

as_array function
3 participants
@Fuuzetsu @Lokathor @LunNova